lib: Always checksum content in deltas

This is a follow up to conversation on list - in practice, if we're
backing away from summary signing, then it makes sense to remove the
special casing for checksums in deltas around summary signatures.

This is also related to the recent change to enable GPG checking for
commits in deltas - now we have a more coherent story between the
previous pull path and deltas.

I didn't do any performance checking, and while it's slightly annoying
that we're now doing sha256 on the delta content twice (once for the
part and once per object)...sha256 is pretty fast, I think most users
are I/O bound anyways, and it'd drop even farther if we started using
openssl.

Closes: #612
Approved by: jlebon

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index ffa387a954042276499f2eff4a07456b8c05ec0b..80c704380c86eb1a3d6551870cb5d54507405d85 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -1010,8 +1010,6 @@ static_deltapart_fetch_on_complete (GObject           *object,
   _ostree_static_delta_part_execute_async (pull_data->repo,
                                            fetch_data->objects,
                                            part,
-                                           /* Trust checksums if summary was gpg signed */
-                                           pull_data->gpg_verify_summary && pull_data->summary_data_sig,
                                            pull_data->cancellable,
                                            on_static_delta_written,
                                            fetch_data);
@@ -1661,7 +1659,6 @@ process_one_static_delta (OtPullData   *pull_data,
       g_autoptr(GBytes) inline_part_bytes = NULL;
       guint64 size, usize;
       guint32 version;
-      const gboolean trusted = pull_data->gpg_verify_summary && pull_data->summary_data_sig;
 
       header = g_variant_get_child_value (headers, i);
       g_variant_get (header, "(u@aytt@ay)", &version, &csum_v, &size, &usize, &objects);
@@ -1731,7 +1728,6 @@ process_one_static_delta (OtPullData   *pull_data,
           _ostree_static_delta_part_execute_async (pull_data->repo,
                                                    fetch_data->objects,
                                                    inline_delta_part,
-                                                   trusted,
                                                    pull_data->cancellable,
                                                    on_static_delta_written,
                                                    fetch_data);

diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c
index 133ab016ee822b09e7546a3b725944bacd48a114..5ef4df907814c06260e7c1884cc6261e4376394e 100644 (file)
--- a/src/libostree/ostree-repo-static-delta-core.c
+++ b/src/libostree/ostree-repo-static-delta-core.c
@@ -435,8 +435,7 @@ ostree_repo_static_delta_execute_offline (OstreeRepo                    *self,
         }
 
       if (!_ostree_static_delta_part_execute (self, objects, part, skip_validation,
-                                              FALSE, NULL,
-                                              cancellable, error))
+                                              NULL, cancellable, error))
         {
           g_prefix_error (error, "Executing delta part %i: ", i);
           goto out;
@@ -653,7 +652,7 @@ show_one_part (OstreeRepo                    *self,
              (guint64)g_variant_n_children (ops));
 
     if (!_ostree_static_delta_part_execute (self, objects,
-                                            part, TRUE, TRUE,
+                                            part, TRUE,
                                             &stats, cancellable, error))
       goto out;
 

diff --git a/src/libostree/ostree-repo-static-delta-private.h b/src/libostree/ostree-repo-static-delta-private.h
index 31e8971ee1a49eaee70a0f795fd69e4a59549c61..6121c4820875e3fd1b79c7620f8e58880a7c213e 100644 (file)
--- a/src/libostree/ostree-repo-static-delta-private.h
+++ b/src/libostree/ostree-repo-static-delta-private.h
@@ -141,7 +141,6 @@ typedef struct {
 gboolean _ostree_static_delta_part_execute (OstreeRepo      *repo,
                                             GVariant        *header,
                                             GVariant        *part_payload,
-                                            gboolean         trusted,
                                             gboolean         stats_only,
                                             OstreeDeltaExecuteStats *stats,
                                             GCancellable    *cancellable,
@@ -150,7 +149,6 @@ gboolean _ostree_static_delta_part_execute (OstreeRepo      *repo,
 void _ostree_static_delta_part_execute_async (OstreeRepo      *repo,
                                               GVariant        *header,
                                               GVariant        *part_payload,
-                                              gboolean         trusted,
                                               GCancellable    *cancellable,
                                               GAsyncReadyCallback  callback,
                                               gpointer         user_data);

diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c
index ff5a8a1a851e6477d51400209f563ef19e2164cc..a71e070fb32bbe8a37ddefcaab8906e08512dac5 100644 (file)
--- a/src/libostree/ostree-repo-static-delta-processing.c
+++ b/src/libostree/ostree-repo-static-delta-processing.c
@@ -39,7 +39,6 @@
 G_STATIC_ASSERT (sizeof (guint) >= sizeof (guint32));
 
 typedef struct {
-  gboolean        trusted;
   gboolean        stats_only;
   OstreeRepo     *repo;
   guint           checksum_index;
@@ -180,7 +179,6 @@ gboolean
 _ostree_static_delta_part_execute (OstreeRepo      *repo,
                                    GVariant        *objects,
                                    GVariant        *part,
-                                   gboolean         trusted,
                                    gboolean         stats_only,
                                    OstreeDeltaExecuteStats *stats,
                                    GCancellable    *cancellable,
@@ -200,7 +198,6 @@ _ostree_static_delta_part_execute (OstreeRepo      *repo,
 
   state->repo = repo;
   state->async_error = error;
-  state->trusted = trusted;
   state->stats_only = stats_only;
 
   if (!_ostree_static_delta_parse_checksum_array (objects,
@@ -296,7 +293,6 @@ typedef struct {
   GVariant *part;
   GCancellable *cancellable;
   GSimpleAsyncResult *result;
-  gboolean trusted;
 } StaticDeltaPartExecuteAsyncData;
 
 static void
@@ -323,7 +319,6 @@ static_delta_part_execute_thread (GSimpleAsyncResult  *res,
   if (!_ostree_static_delta_part_execute (data->repo,
                                           data->header,
                                           data->part,
-                                          data->trusted,
                                           FALSE, NULL,
                                           cancellable, &error))
     g_simple_async_result_take_error (res, error);
@@ -333,7 +328,6 @@ void
 _ostree_static_delta_part_execute_async (OstreeRepo      *repo,
                                          GVariant        *header,
                                          GVariant        *part,
-                                         gboolean         trusted,
                                          GCancellable    *cancellable,
                                          GAsyncReadyCallback  callback,
                                          gpointer         user_data)
@@ -344,7 +338,6 @@ _ostree_static_delta_part_execute_async (OstreeRepo      *repo,
   asyncdata->repo = g_object_ref (repo);
   asyncdata->header = g_variant_ref (header);
   asyncdata->part = g_variant_ref (part);
-  asyncdata->trusted = trusted;
   asyncdata->cancellable = cancellable ? g_object_ref (cancellable) : NULL;
 
   asyncdata->result = g_simple_async_result_new ((GObject*) repo,
@@ -517,9 +510,9 @@ dispatch_bspatch (OstreeRepo                 *repo,
   return ret;
 }
 
-/* When processing untrusted static deltas, we need to checksum the
- * file content, which includes a header.  Compare with what
- * ostree_checksum_file_from_input() is doing too.
+/* Before, we had a distinction between "trusted" and "untrusted" deltas
+ * which we've decided wasn't a good idea.  Now, we always checksum the content.
+ * Compare with what ostree_checksum_file_from_input() is doing too.
  */
 static gboolean
 handle_untrusted_content_checksum (OstreeRepo                 *repo,
@@ -530,13 +523,10 @@ handle_untrusted_content_checksum (OstreeRepo                 *repo,
   g_autoptr(GVariant) header = NULL;
   g_autoptr(GFileInfo) finfo = NULL;
   gsize bytes_written;
-  
-  if (state->trusted)
-    return TRUE;
 
   finfo = _ostree_header_gfile_info_new (state->mode, state->uid, state->gid);
   header = _ostree_file_header_new (finfo, state->xattrs);
-  
+
   state->content_checksum = g_checksum_new (G_CHECKSUM_SHA256);
 
   if (!_ostree_write_variant_with_size (NULL, header, 0, &bytes_written, state->content_checksum,
@@ -579,26 +569,16 @@ dispatch_open_splice_and_close (OstreeRepo                 *repo,
       metadata = g_variant_new_from_data (ostree_metadata_variant_type (state->output_objtype),
                                           state->payload_data + offset, length, TRUE, NULL, NULL);
 
-      if (state->trusted)
-        {
-          if (!ostree_repo_write_metadata_trusted (state->repo, state->output_objtype,
-                                                   state->checksum,
-                                                   metadata,
-                                                   cancellable,
-                                                   error))
-            goto out;
-        }
-      else
-        {
-          g_autofree guchar *actual_csum = NULL;
+      {
+        g_autofree guchar *actual_csum = NULL;
 
-          if (!ostree_repo_write_metadata (state->repo, state->output_objtype,
-                                           state->checksum,
-                                           metadata, &actual_csum,
-                                           cancellable,
-                                           error))
-            goto out;
-        }
+        if (!ostree_repo_write_metadata (state->repo, state->output_objtype,
+                                         state->checksum,
+                                         metadata, &actual_csum,
+                                         cancellable,
+                                         error))
+          goto out;
+      }
     }
   else
     {
@@ -672,29 +652,18 @@ dispatch_open_splice_and_close (OstreeRepo                 *repo,
                                                   &object_input, &objlen,
                                                   cancellable, error))
             goto out;
-          
-          if (state->trusted)
-            {
-              if (!ostree_repo_write_content_trusted (state->repo,
-                                                      state->checksum,
-                                                      object_input,
-                                                      objlen,
-                                                      cancellable,
-                                                      error))
-                goto out;
-            }
-          else
-            {
-              g_autofree guchar *actual_csum = NULL;
-              if (!ostree_repo_write_content (state->repo,
-                                              state->checksum,
-                                              object_input,
-                                              objlen,
-                                              &actual_csum,
-                                              cancellable,
-                                              error))
-                goto out;
-            }
+
+          {
+            g_autofree guchar *actual_csum = NULL;
+            if (!ostree_repo_write_content (state->repo,
+                                            state->checksum,
+                                            object_input,
+                                            objlen,
+                                            &actual_csum,
+                                            cancellable,
+                                            error))
+              goto out;
+          }
         }
     }
 
@@ -920,8 +889,6 @@ dispatch_close (OstreeRepo                 *repo,
         {
           const char *actual_checksum = g_checksum_get_string (state->content_checksum);
 
-          g_assert (!state->trusted);
-
           if (strcmp (actual_checksum, state->checksum) != 0)
             {
               g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,